-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(dex): solve token left in dex contract issue #8
Conversation
Codecov Report
@@ Coverage Diff @@
## build_pancake_dex #8 +/- ##
=====================================================
- Coverage 36.36% 36.17% -0.20%
=====================================================
Files 14 16 +2
Lines 319 329 +10
Branches 33 34 +1
=====================================================
+ Hits 116 119 +3
- Misses 194 200 +6
- Partials 9 10 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
import "../../libraries/Errors.sol"; | ||
|
||
abstract contract BasicDex { | ||
modifier afterSwapCheck(address _sellToken, address _buyToken) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the modifier being used anywhere. Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thank you for the catch. I've added them to the actual DEX. I was only testing it in the mockDex.
* chore: change test helper file name * build: implement basicDex with tokenWithdraw feature * test: mock basic dex * fix: add token withdraw to all dexes * chore: adjust input type to calldata * build: add invalid balance error * test: add afterswap test * refactor: add token check modifier to doSwap * chore: consistent between optimizer * fix: solve stack too deep issue * fix: disable optimizer in test
Summary
In this pull request we aim to solve the issue that sell token might left in dex contract.
Details
The main changes in this pull request is: